feat(port): Implement GLOBALLY_UNIQUE
#5723
Conversation
Co-Authored-By: anothersimulacrum <[email protected]>
github-actions
bot
added
src
|
Autofix has formatted code style violation in this PR. I edit commits locally (e.g: git, github desktop) and want to keep autofix
I do not want the automated commit
If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT. |
Generally speaking I'm ok with adding more specific military locations and tying them into quests than just having a single generic military base. If we want to go with players being able to access military loot sustainably we can add national guard armories and an airbase, plus garrisons. The carrier is a bit of a weird one as finding several carriers in lakes would be more of a stretch than finding one. |
those were just from the original dda pr, but there are probably a few other locations that could be Globally Unique
edit: I removed the flag from the military base, because it isn't unique enough to be a one-off location |
|
Testing in build
Testing in compiled PR:
Both tests done with default world settings. Only having it fail to generate once is arguably not fatal to this PR since the sample size is low enough and the carrier scenario is suspected to do this occasionally even without this PR, but the crash is definitely concerning. |
|
I've yet to get it to crash again to try and grab more info, but I did just get four failures to generate in a row. |
chaosvolt
dismissed
scarf005’s stale review
It's being wacky and Kheir said in the discord it has lots of unndeeded code that needs to be stripped out, so this approval was definitely premature
| if( special.has_flag( "UNIQUE" ) ) { | ||
| if( unique || globally_unique ) { | ||
| const overmap_special_id &id = iter.special_details->id; | ||
| const overmap_special_placement_constraints &constraints = iter.special_details->get_constraints(); |
There was a problem hiding this comment.
Where do these variables come into play? Remove, or incorporate them into the code. Check the Files Changed tab to check for these variables (I count at least 4).
It shouldn't really even have passed tests with unused variables. Did we disable that?
There was a problem hiding this comment.
It shouldn't really even have passed tests with unused variables. Did we disable that?
lint for unused variables exists but it only emits warning because it often crashed on unrelated existing issues
There was a problem hiding this comment.
speaking of "unrelated", should make it error again since #5548 is merged and there probably won't be unrelated changes anymore
Checklist
Required
mainso it won't cause conflict when updatingmainbranch later.Optional
Co-Authored-Byin the commit message.doc/folder.-->
Purpose of change
ports dda change
Describe the solution
implements
GLOBALLY_UNIQUEflag that makes overmap specials exist only once.Describe alternatives you've considered
Testing
Made multiple characters spawn at the aircraft carrier and military base, all characters spawned at the same locations in the world. Also made those characters get the "Go to the Evac Center" mission, and it directed the characters to the same evac center.
Additional context